fix: complete deterministic rollout samplingPyq/complete determinism concurrent rollout - #1607
Conversation
| if "top_p" not in kwargs: | ||
| kwargs["top_p"] = 1.0 | ||
|
|
||
| deterministic_sampling = session is not None and get_bool_env_var( |
There was a problem hiding this comment.
The issue is that the command to start Data Proxy did not pass this configuration or environment variable, so the V2 requests still will not automatically generate a seed, and the subsequent SGLang sampling_seed forwarding also cannot obtain the value.
Suggestion: Add deterministic_sampling to DataProxyConfig and startup parameters, or explicitly pass the environment variable when forking Data Proxy.
There was a problem hiding this comment.
Thanks for catching this. The original PR missed the explicit propagation from the public InferenceEngineConfig to the forked V2 Data Proxy.
This did not surface in our local deterministic baseline because its runner/YAML explicitly exported AREAL_DETERMINISTIC_SAMPLING=1, and the RPC Guard copied the parent environment when forking the Data Proxy. The runtime seed logs confirmed that deterministic seed derivation was active in those runs. However, that baseline-specific environment inheritance was not a valid configuration contract for general users.
Addressed in efef650:
- Added deterministic_sampling: bool = False to DataProxyConfig.
- Added the corresponding Data Proxy CLI option.
- RolloutControllerV2 now explicitly passes --deterministic-sampling when enabled.
- The Data Proxy request path now reads app.state.config.deterministic_sampling instead of the environment variable.
- Added tests covering enabled/disabled propagation, deterministic seed derivation, and explicit-seed precedence.
|
The vllm-related part has not been fixed in the related part. I understand that deterministic_sampling is a backend-independent common configuration. If convenient, can you include the vllm part update and supplement the relevant UT? |
| task_id in self._pending_results for task_id in task_frontier | ||
| ) | ||
| else: | ||
| results_ready = len(self._pending_results) >= count |
There was a problem hiding this comment.
In the synchronous, no-rejection case, the staleness manager allows only one
consumer batch to run per model version. Later task IDs cannot complete before
the current batch, so sorting completed results by task ID and disabling
shuffle appears sufficient; the frozen membership frontier seems redundant.
Is the frontier intended specifically for rejection/timeout or
submit-many/wait-few scenarios? If so, could that behavior be scoped and tested
separately?
There was a problem hiding this comment.
Agreed. The frontier was originally intended as a defensive mechanism for broader dispatcher scenarios such as submit-many/wait-few with out-of-order completion. However, after rechecking the supported synchronous contract and the baseline evidence, we found that this additional membership guarantee was neither required nor validated, and it could introduce head-of-line blocking.
I removed the submission-order frontier in efef650. Deterministic mode now waits for any count completed results, sorts those completed results by task ID, and does not shuffle them. The default path retains its existing create-time selection and shuffle behavior.
I also updated the tests to cover deterministic ordering without shuffle, default-path shuffling, and the case where an earlier task remains unfinished while later completed tasks are returned.
| deterministic_sampling: bool = field( | ||
| default=False, | ||
| metadata={ |
There was a problem hiding this comment.
Could you clarify the determinism contract for
max_head_offpolicyness > 0?
If asynchronous staleness is supported, how does this implementation guarantee
a stable task-to-weight-version mapping across runs? The inference version is
read when a generation request is actually sent, so the same logical task may
use different model versions depending on scheduling timing. Stable seeds,
result ordering, and a membership frontier do not appear to fix that.
If end-to-end determinism is only supported with
max_head_offpolicyness=0, should the configuration emit a warning or document
that requirement explicitly?
There was a problem hiding this comment.
You’re right — this implementation does not guarantee a stable task-to-weight-version mapping when max_head_offpolicyness > 0. End-to-end determinism is currently supported only with max_head_offpolicyness=0.
Addressed in efef650:
- InferenceEngineConfig now emits a warning when deterministic_sampling=True and max_head_offpolicyness>0.
- The CLI help explicitly documents the max_head_offpolicyness=0 requirement.
- The membership frontier was removed because stable seeds and result ordering cannot fix scheduling-dependent version assignment.
- Tests cover the warning and non-warning configurations.
Supporting deterministic asynchronous rollout would require a separate contract, likely binding each task to a model version at submission time and defining retry/rejection behavior around that binding. That is outside the scope of this PR.
| logger.info( | ||
| "V2 rollout member start: task_id=%s group_id=%s member=%d " | ||
| "session_id=%s version=%s mode=%s", | ||
| task_id, | ||
| group_id, | ||
| member_index, | ||
| session_id, | ||
| version, | ||
| execution_mode, | ||
| ) |
There was a problem hiding this comment.
Could the per-member start/finish messages be moved to DEBUG or gated by
enable_rollout_tracing? This emits two INFO lines per trajectory plus one line
per group. With batch_size=16 and n_samples=8 that is at least 272 additional
INFO lines per training step.
There was a problem hiding this comment.
Addressed in efef650. The per-member start/finish messages have been moved from INFO to DEBUG, while the single group-level dispatch message remains at INFO.
This reduces the default INFO log volume from trajectory scale to group scale, while preserving detailed member-level diagnostics when DEBUG logging is enabled.
| @@ -2098,6 +2102,7 @@ class SGLangConfig: | |||
| enable_memory_saver: bool = False | |||
| allow_auto_truncate: bool = False | |||
| attention_backend: str | None = "fa3" | |||
There was a problem hiding this comment.
SGLang documents deterministic inference support only for the flashinfer, fa3,
and triton attention backends. At the moment enable_deterministic_inference is
forwarded for any configured backend, which can give users a false
determinism guarantee.
Could we emit a warning when deterministic inference is enabled with an
explicit attention_backend outside {flashinfer, fa3, triton}? None can remain
allowed because it delegates to the SGLang default.
There was a problem hiding this comment.
Good point — addressed in efef650.
SGLangConfig.build_args() now emits a warning when enable_deterministic_inference=True and an explicitly configured attention_backend is outside {flashinfer, fa3, triton}. attention_backend=None remains allowed so SGLang can select its default backend.
I also added tests covering all documented backends, None, and an explicitly unsupported backend.
| if self.serialize_group_samples: | ||
| results = [] | ||
| for member_index, (session_id, session_api_key) in enumerate(sessions): | ||
| results.append( | ||
| await _run_one(member_index, session_id, session_api_key) | ||
| ) | ||
| else: | ||
| results = await asyncio.gather( | ||
| *[ | ||
| _run_one(member_index, session_id, session_api_key) | ||
| for member_index, (session_id, session_api_key) in enumerate( | ||
| sessions | ||
| ) | ||
| ] | ||
| ) |
There was a problem hiding this comment.
I don't think serializing members within one group is sufficient to stabilize
SGLang batch composition. Multiple prompt groups still execute concurrently,
so a serialized member from group A can be co-batched with requests from group
B in timing-dependent ways.
This guarantees per-group member order, but not “strict reproducibility” of
dynamic batching. Could we remove or weaken this claim and rely on SGLang
batch-invariant inference instead? A true serialization fallback would need a
global request scheduler, not a per-group loop.
There was a problem hiding this comment.
Agreed. This is separate from the submission-order frontier: the frontier only affected result collection after generation and could not stabilize SGLang batch composition, so it has been removed.
We retain serialize_group_samples because it provides the within-group 0 -> 1 -> 2 -> 3 submission order used by our strict-gate baseline. However, we agree that it does not serialize requests across concurrently running groups and therefore is not, by itself, a strict-reproducibility guarantee.
In efef650, we narrowed the help text and tests to this within-group contract. Concurrent deterministic generation relies on SGLang’s batch-invariant deterministic inference. A fully serialized fallback would require a global request scheduler, which is outside the scope of this PR.
Derive stable sampling seeds for OpenAI proxy sessions and preserve canonical rollout group order while inference requests run concurrently. Consume completed work through a submission-order frontier so rollout completion timing cannot change training batch membership. Forward request seeds and deterministic-inference configuration to SGLang, and bind callbacks after task ID allocation.
1f5099b to
efef650
Compare
| @@ -562,6 +607,7 @@ def wait_results( | |||
| ------- | |||
| list[TResult | None] | |||
| List of task results, None for rejected tasks. | |||
|
|
|||
| """ | |||
| if count <= 0: | |||
| raise ValueError(f"count must be positive, got {count}") | |||
| @@ -573,6 +619,8 @@ def wait_results( | |||
| with self._result_cv: | |||
| while len(self._pending_results) < count: | |||
| self._check_thread_exception() | |||
| if self._shutdown_event.is_set(): | |||
| raise RuntimeError("Task dispatcher is shutting down") | |||
|
|
|||
| elapsed = time.perf_counter() - start_time | |||
| remaining = timeout - elapsed | |||
| @@ -588,18 +636,17 @@ def wait_results( | |||
|
|
|||
| drained: list[TimedResult[TResult]] = list(self._pending_results.values()) | |||
| self._pending_results.clear() | |||
There was a problem hiding this comment.
These callback, task-ID, enqueue rollback, and dynamic-batch changes appear
independent of deterministic sampling. Could we move them and their tests to a
separate dispatcher correctness PR, and keep this PR focused on deterministic
seed propagation and result ordering?
Summary
Complete deterministic rollout sampling across concurrent rollout scheduling and the V1/V2 inference paths.
This PR makes rollout identity, request seed assignment, and result ordering reproducible while preserving concurrent execution by default. It also provides an explicit opt-in serial mode for strict V2 reproducibility when SGLang dynamic batching affects numerical identity.
Motivation
Deterministic inference requires more than enabling deterministic kernels on the SGLang server.
Previously, several gaps remained:
seedcould reachArealOpenAIwithout being carried through the complete request path;GenerationHyperparameters.seedas SGLangsampling_seed;asyncio.gather, so their SGLang dynamic-batch composition could vary between runs.Together, these gaps could make repeated runs diverge even when the global training seed and SGLang deterministic inference were enabled.
Changes
Stable concurrent rollout identity and ordering
End-to-end sampling seed propagation
The request path is now:
This path is covered for:
Explicit caller-provided seeds always take precedence over automatically derived seeds.
Shared seed derivation
Add a shared derive_deterministic_seed(identity, request_index) helper so V1 and V2 use the same stable derivation logic.
The generated seed is:
Optional strict V2 group serialization
Add the explicit configuration:
InferenceEngineConfig.serialize_group_samples: bool = False
Behavior:
False:
samples within a V2 group continue to run concurrently with asyncio.gather
True:
samples run sequentially in stable member order
member 0 -> member 1 -> member 2 -> ...
This option is intentionally independent from deterministic_sampling.
deterministic_sampling stabilizes identities, seeds, scheduling, and result ordering while retaining concurrency. serialize_group_samples additionally stabilizes request arrival and SGLang batching conditions for strict reproducibility, at the expected cost of rollout throughput.
The default remains concurrent, so existing workloads are unaffected.
Observability
V2 workflow logs now include:
These fields make it possible to verify the effective rollout execution path from runtime logs.
Failure handling
Serial execution preserves the existing group-cleanup behavior:
Documentation
Regenerate the English and Chinese CLI references with the new serialize_group_samples option.
Compatibility
All deterministic behavior remains opt-in:
Testing
Focused tests were executed in the project Slurm training image:
tests/test_deterministic_sampling.py
tests/v2/inference_service/test_controller.py
tests/v2/inference_service/test_data_proxy_chat.py
118 passed, 4 skipped
The tests cover:
Additional checks:
Ruff lint/format: passed
mdformat: passed
git diff --check: passed
Python compilation: passed
CLI documentation generation: passed
The remaining warning is an existing third-party torchao SyntaxWarning.